Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: カードを削除したときにcard_listが二重削除されていた問題を修正 #93

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

yumetodo
Copy link
Contributor

@yumetodo yumetodo commented Jan 13, 2020

#62 でArray#spliceに書き換えたときに消し忘れた

resolve #92

ref:

@sasanquaneuf
Copy link
Contributor

二重削除問題はよさそうですけど、flipと組み合わせるとおかしくなりました。
添付画像参照
2,1,1,2になってほしい
なにかおかしい

@sasanquaneuf
Copy link
Contributor

1,1,2,2, -(flip)-> 2,2,1,1, -(add 2)-> 2,2,1,1,2, -(delete 2つ目の2)-> 2,2,1,2
(本当は2,1,1,2になってほしい)
みたいな手順で再現します。最初の1,1,2,2,は順番に追加しています。

@yumetodo
Copy link
Contributor Author

これはflipの実装がクソっぽい。要素削除はidベースなのにそれを考えてない #81 のせい。

@sasanquaneuf
Copy link
Contributor

なるほど、flipと組み合わせる件はとりあえず別issueでもいいかなと思います(二重削除はdelete単体の問題なので、これがマージされたら改善するという意味で)
二重削除自体はLGTM

@yumetodo
Copy link
Contributor Author

別の問題なので当PRでは直しません。 #94

@KEINOS
Copy link
Member

KEINOS commented Jan 13, 2020

Chrome 79.0.3945.117 @ Mac で二重削除されないことを確認しました。LGTM だと思います。

特に他に影響しないと思うのでマージします。

Closing #92 (cc @yume-yu @blhsrwznrghfzpr )

YeiYeiYei

@KEINOS KEINOS merged commit 3f7ea5e into Qithub-BOT:master Jan 13, 2020
@yumetodo yumetodo deleted the patch-1 branch January 13, 2020 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants